Skip to content

Conversation

@itamaro
Copy link
Contributor

@itamaro itamaro commented Sep 21, 2025

Copy link
Member

@encukou encukou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks reasonable.

I'm proposing a replacement for Init functions in PEP-793, but that PEP adds PyModule_FromSlotsAndSpec and PyModule_Exec which should work for similar use cases.
Would it make sense to switch the spec & initfunc arguments to match that proposal?

I'm not sure if the word Builtin in the name is necessary -- is that to match the internal create_builtin? Would something like PyImport_LoadModuleFromInitfuncAndSpec work better?

Do you want to submit this to the C API WG?

@itamaro
Copy link
Contributor Author

itamaro commented Sep 23, 2025

Would it make sense to switch the spec & initfunc arguments to match that proposal?

I think it should be fine! Is the PEP already implemented on main?

I'm not sure if the word Builtin in the name is necessary -- is that to match the internal create_builtin? Would something like PyImport_LoadModuleFromInitfuncAndSpec work better?

Yes, I used it to reflect what it does internally. PyImport_LoadModuleFromInitfuncAndSpec sounds great to me!

Do you want to submit this to the C API WG?

Will do. Should we first decide whether to change this to match PEP 793, or go to the WP with options and get their input?

@encukou
Copy link
Member

encukou commented Sep 24, 2025

Is the PEP already implemented on main?

No, it's not even accepted :)
I have a slight preference to align the two proposals, in case both make it. I'm checking if you have strong reasons for the order here. No need to update the PR now.

Will do. Should we first decide whether to change this to match PEP 793, or go to the WP with options and get their input?

Just go to the WG :)

@itamaro
Copy link
Contributor Author

itamaro commented Sep 24, 2025

Just go to the WG :)

done :) capi-workgroup/decisions#77

@itamaro itamaro force-pushed the gh-116146-create-builtin-initfunc branch from 4363e5a to 781a730 Compare November 8, 2025 18:52
- remove unused `found` variable
- use `my_test_extension` instead of `embedded_ext` (the former is free-threading-ready, the latter prints a warning)
@itamaro itamaro changed the title gh-116146: Add new C-API to create builtin from spec and initfunc gh-116146: Add C-API to create module from spec and initfunc Nov 9, 2025
@itamaro itamaro marked this pull request as ready for review November 9, 2025 01:01
@encukou
Copy link
Member

encukou commented Nov 11, 2025

I left my suggestions as a PR: itamaro#27

* Test single-phase init as well; don't use private APIs in test
* Doc update

---------

Co-authored-by: Itamar Oren <[email protected]>
@python-cla-bot
Copy link

python-cla-bot bot commented Nov 12, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

@itamaro itamaro force-pushed the gh-116146-create-builtin-initfunc branch from 8f16533 to 1165950 Compare November 12, 2025 16:18
@itamaro
Copy link
Contributor Author

itamaro commented Nov 12, 2025

thank you @encukou @vstinner @kumaraditya303 for the review and suggestions!

@encukou are you planning to merge gh-141197 soon? until that is merged, this PR has a docs failure due to referring to the function that you document in gh-141197.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new function PyImport_CreateModuleFromInitfunc() should be documented in Doc/whatsnew/3.15.rst.

Co-authored-by: Petr Viktorin <[email protected]>
Co-authored-by: Kumar Aditya <[email protected]>
Co-authored-by: Victor Stinner <[email protected]>
@itamaro itamaro requested a review from AA-Turner as a code owner November 13, 2025 15:31
@itamaro itamaro force-pushed the gh-116146-create-builtin-initfunc branch from fabafb5 to fbfde0a Compare November 13, 2025 15:32
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the updates!

@encukou encukou merged commit 1e4e59b into python:main Nov 14, 2025
46 checks passed
encukou added a commit to encukou/cpython that referenced this pull request Nov 14, 2025
@vstinner
Copy link
Member

Congrats @itamaro!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants